Add stop thread keybinding command#4308
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7ae0496. Configure here.
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
| void onInterrupt(); | ||
| return; |
There was a problem hiding this comment.
Stop shortcut ignores running state
Medium Severity
The new thread.stop keybinding unconditionally calls onInterrupt. Unlike the UI stop button, which is only active when a turn is running, this allows interrupting idle threads or drafts without an active server turn, which can lead to unexpected thread errors or interrupt failure messages.
Reviewed by Cursor Bugbot for commit 7ae0496. Configure here.
| if (command === "thread.stop") { | ||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
| void onInterrupt(); |
There was a problem hiding this comment.
Held stop key repeats interrupt
Low Severity
The thread.stop keydown branch does not ignore event.repeat, so holding the bound key can enqueue multiple onInterrupt calls. Other thread-related shortcuts in the sidebar explicitly bail out when event.repeat is true.
Reviewed by Cursor Bugbot for commit 7ae0496. Configure here.
ApprovabilityVerdict: Needs human review This PR adds new user-facing keybinding functionality with unresolved review comments identifying potential bugs: the stop shortcut doesn't check if a thread is running before attempting interrupt, and doesn't handle key repeat events. These issues warrant human review. You can customize Macroscope's approvability policy. Learn more. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ae04963d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (command === "thread.stop") { | ||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
| void onInterrupt(); |
There was a problem hiding this comment.
When a custom thread.stop shortcut is pressed on an active thread after its turn has settled, this block still calls onInterrupt() even though the provider session may only be ready. The server accepts thread.turn.interrupt for any non-stopped session and adapters such as OpenCode abort the session without requiring an active turn, while the existing Stop button is only exposed when phase === "running". Gate this shortcut on the same running/activeTurnId condition so pressing the binding on an idle thread is a no-op instead of aborting provider state.
Useful? React with 👍 / 👎.
1 participant
Summary
Screen.Recording.2026-07-22.at.4.36.45.PM.mov
thread.stopkeybinding command without assigning a default shortcutTesting
vp test run packages/contracts/src/keybindings.test.ts apps/web/src/keybindings.test.ts apps/web/src/components/settings/KeybindingsSettings.logic.test.ts(58 tests)vp run typecheckinpackages/contractsvp run typecheckinapps/webvp fmt --checkandvp lintfor all changed filesthread.stopbinding; the turn returned toreadywith no active turn or errorNote
Add
thread.stopkeybinding command to interrupt the active thread turnthread.stoptoTHREAD_KEYBINDING_COMMANDSin the contracts package and exportsSTATIC_KEYBINDING_COMMANDSfor use by consumers.keydownhandler inChatViewContentthat invokesonInterruptwhen thethread.stopcommand fires, surfacing failures viasetThreadError.buildKeybindingCommandOptionsto seed fromSTATIC_KEYBINDING_COMMANDSinstead ofDEFAULT_RESOLVED_KEYBINDINGS, ensuringthread.stopappears in the keybindings settings UI even though it has no default binding.Macroscope summarized 7ae0496.
Note
Low Risk
Reuses existing turn-interrupt RPC and UI error handling; no default shortcut and no auth or data-model changes.
Overview
Adds a typed
thread.stopkeybinding command so users can stop the current agent turn from a custom shortcut (there is no default binding).Contracts:
thread.stopis included inTHREAD_KEYBINDING_COMMANDS, andSTATIC_KEYBINDING_COMMANDSis exported for the web app.Chat:
onInterruptis a stableuseCallbackthat callsinterruptThreadTurnand surfaces failures viasetThreadError. The global keydown handler runsthread.stopthrough that same path as the composer stop control.Settings: The keybinding command picker seeds from
STATIC_KEYBINDING_COMMANDSinstead of only default resolved bindings, so unbound commands likethread.stopstill appear for assignment.Tests cover schema decoding, shortcut resolution, and command-option building.
Reviewed by Cursor Bugbot for commit 7ae0496. Bugbot is set up for automated code reviews on this repo. Configure here.